From 17ddbb2c7235709d5708119ce277d48d381c2cdd Mon Sep 17 00:00:00 2001 From: tsteven4 <13596209+tsteven4@users.noreply.github.com> Date: Tue, 28 Apr 2020 06:42:48 -0600 Subject: [PATCH] introduce xasprintf that accepts a QScopedPointer& (#547) and use it to fix gcc 9 warnings in magproto, as well as other possible string truncations/overflows. --- defs.h | 48 ++++++++++++++++--------------- magproto.cc | 82 ++++++++++++++++++++++++++--------------------------- util.cc | 73 +++++++++++++++++++++++++++++------------------ 3 files changed, 111 insertions(+), 92 deletions(-) diff --git a/defs.h b/defs.h index 5d57a95f3..1fcfb56cc 100644 --- a/defs.h +++ b/defs.h @@ -19,38 +19,39 @@ #ifndef DEFS_H_INCLUDED_ #define DEFS_H_INCLUDED_ -#include // for sort, stable_sort -#include // for M_PI -#include // for va_list -#include // for NULL, nullptr_t, size_t -#include // for int32_t, uint32_t -#include // for NULL, fprintf, FILE, stdout -#include // for time_t -#include // for move +#include // for sort, stable_sort +#include // for M_PI +#include // for va_list +#include // for NULL, nullptr_t, size_t +#include // for int32_t, uint32_t +#include // for NULL, fprintf, FILE, stdout +#include // for time_t +#include // for move #if HAVE_CONFIG_H #include "config.h" #endif #if HAVE_LIBZ -#include // doesn't really belong here, but is missing elsewhere. +#include // doesn't really belong here, but is missing elsewhere. #elif !ZLIB_INHIBITED -#include "zlib.h" // doesn't really belong here, but is missing elsewhere. +#include "zlib.h" // doesn't really belong here, but is missing elsewhere. #endif -#include // for QList, QList<>::const_reverse_iterator, QList<>::reverse_iterator -#include // for QString -#include // for QStringRef -#include // for QTextCodec -#include // for QVector -#include // for CaseInsensitive -#include // for foreach +#include // for QList, QList<>::const_reverse_iterator, QList<>::reverse_iterator +#include // for QScopedPointer +#include // for QString +#include // for QStringRef +#include // for QTextCodec +#include // for QVector +#include // for CaseInsensitive +#include // for foreach -#include "formspec.h" // for FormatSpecificData -#include "inifile.h" // for inifile_t -#include "gbfile.h" // doesn't really belong here, but is missing elsewhere. -#include "session.h" // for session_t -#include "src/core/datetime.h" // for DateTime -#include "src/core/optional.h" // for optional +#include "formspec.h" // for FormatSpecificData +#include "inifile.h" // for inifile_t +#include "gbfile.h" // doesn't really belong here, but is missing elsewhere. +#include "session.h" // for session_t +#include "src/core/datetime.h" // for DateTime +#include "src/core/optional.h" // for optional #define CSTR(qstr) ((qstr).toUtf8().constData()) @@ -1108,6 +1109,7 @@ void rtrim(char* s); char* lrtrim(char* buff); int xasprintf(char** strp, const char* fmt, ...) PRINTFLIKE(2, 3); int xasprintf(QString* strp, const char* fmt, ...) PRINTFLIKE(2, 3); +int xasprintf(QScopedPointer& strp, const char* fmt, ...) PRINTFLIKE(2, 3); int xvasprintf(char** strp, const char* fmt, va_list ap); char* strupper(char* src); char* strlower(char* src); diff --git a/magproto.cc b/magproto.cc index 1df7fcd21..3b917c6d8 100644 --- a/magproto.cc +++ b/magproto.cc @@ -22,7 +22,7 @@ #include // for isprint, toupper #include // for fabs, lround -#include // for sprintf, sscanf, snprintf, size_t +#include // for sscanf, size_t #include // for atoi, atof, strtoul #include // for strchr, strncmp, strlen, memmove, strrchr, memset #include // for gmtime @@ -34,6 +34,7 @@ #include // for QFileInfoList #include // for QLatin1String #include // for QList +#include // for QScopedPointer #include // for QString, operator== #include // for QStringList #include // for QTime @@ -61,7 +62,7 @@ static int broken_sportrak; #define debug_serial (global_opts.debug_level > 1) static QString termread(char* ibuf, int size); -static void termwrite(char* obuf, int size); +static void termwrite(const char* obuf, int size); static void mag_readmsg(gpsdata_type objective); static void mag_handon(); static void mag_handoff(); @@ -313,7 +314,7 @@ mag_writemsg(const char* const buf) { unsigned int osum = mag_checksum(buf); int retry_cnt = 5; - char obuf[1000]; + QScopedPointer obuf; if (debug_serial) { warning("WRITE: $%s*%02X\r\n",buf, osum); @@ -321,8 +322,8 @@ mag_writemsg(const char* const buf) retry: - int i = sprintf(obuf, "$%s*%02X\r\n",buf, osum); - termwrite(obuf, i); + int i = xasprintf(obuf, "$%s*%02X\r\n",buf, osum); + termwrite(obuf.data(), i); if (magrxstate == mrs_handon || magrxstate == mrs_awaiting_ack) { magrxstate = mrs_awaiting_ack; mag_readmsg(trkdata); @@ -345,25 +346,25 @@ retry: static void mag_writeack(int osum) { - char obuf[200]; - char nbuf[200]; + QScopedPointer nbuf; + QScopedPointer obuf; if (is_file) { return; } - (void) sprintf(nbuf, "PMGNCSM,%02X", osum); - unsigned int nsum = mag_checksum(nbuf); - int i = sprintf(obuf, "$%s*%02X\r\n",nbuf, nsum); + (void) xasprintf(nbuf, "PMGNCSM,%02X", osum); + unsigned int nsum = mag_checksum(nbuf.data()); + int i = xasprintf(obuf, "$%s*%02X\r\n",nbuf.data(), nsum); if (debug_serial) { - warning("ACK WRITE: %s",obuf); + warning("ACK WRITE: %s",obuf.data()); } /* * Don't call mag_writemsg here so we don't get into ack feedback * loops. */ - termwrite(obuf, i); + termwrite(obuf.data(), i); } static void @@ -656,7 +657,7 @@ mag_dequote(char* ibuf) } static void -termwrite(char* obuf, int size) +termwrite(const char* obuf, int size) { if (is_file) { size_t nw; @@ -1330,8 +1331,8 @@ static void mag_waypt_pr(const Waypoint* waypointp) { - char obuf[200]; - char ofmtdesc[200]; + QScopedPointer obuf; + QScopedPointer ofmtdesc; QString icon_token; double ilat = waypointp->latitude; @@ -1368,9 +1369,9 @@ mag_waypt_pr(const Waypoint* waypointp) if (global_opts.smart_icons && waypointp->gc_data->diff && waypointp->gc_data->terr) { // It's a string and compactness counts, so "1.0" is OK to be "10". - sprintf(ofmtdesc, "%ud/%ud %s", waypointp->gc_data->diff, + xasprintf(ofmtdesc, "%ud/%ud %s", waypointp->gc_data->diff, waypointp->gc_data->terr, CSTRc(odesc)); - odesc = mag_cleanse(ofmtdesc); + odesc = mag_cleanse(ofmtdesc.data()); } else { odesc = mag_cleanse(CSTRc(odesc)); } @@ -1381,10 +1382,10 @@ mag_waypt_pr(const Waypoint* waypointp) * cap on the comments delivered so we leave space for it to route. */ if (!odesc.isEmpty() && (wptcmtcnt++ >= wptcmtcnt_max)) { - odesc[0] = 0; + odesc.clear(); } - sprintf(obuf, "PMGNWPL,%4.3f,%c,%09.3f,%c,%07.0f,M,%-.*s,%-.46s,%s", + xasprintf(obuf, "PMGNWPL,%4.3f,%c,%09.3f,%c,%07.0f,M,%-.*s,%-.46s,%s", lat, ilat < 0 ? 'S' : 'N', lon, ilon < 0 ? 'W' : 'E', waypointp->altitude == unknown_alt ? @@ -1393,11 +1394,11 @@ mag_waypt_pr(const Waypoint* waypointp) CSTRc(owpt), CSTRc(odesc), CSTR(icon_token)); - mag_writemsg(obuf); + mag_writemsg(obuf.data()); if (!is_file) { if (mag_error) { - warning("Protocol error Writing '%s'\n", obuf); + warning("Protocol error Writing '%s'\n", obuf.data()); } } } @@ -1405,7 +1406,7 @@ mag_waypt_pr(const Waypoint* waypointp) static void mag_track_disp(const Waypoint* waypointp) { - char obuf[200]; + QScopedPointer obuf; int hms=0; int fracsec=0; int date=0; @@ -1441,13 +1442,13 @@ void mag_track_disp(const Waypoint* waypointp) lon = (lon_deg * 100.0 + lon); lat = (lat_deg * 100.0 + lat); - sprintf(obuf,"PMGNTRK,%4.3f,%c,%09.3f,%c,%05.0f,%c,%06d.%02d,A,,%06d", + xasprintf(obuf,"PMGNTRK,%4.3f,%c,%09.3f,%c,%05.0f,%c,%06d.%02d,A,,%06d", lat, ilat < 0 ? 'S' : 'N', lon, ilon < 0 ? 'W' : 'E', waypointp->altitude == unknown_alt ? 0 : waypointp->altitude, - 'M',hms,fracsec,date); - mag_writemsg(obuf); + 'M', hms, fracsec, date); + mag_writemsg(obuf.data()); } static @@ -1472,9 +1473,10 @@ and to replace the 2nd name with "<<>>", but I haven't seen one of those. static void mag_route_trl(const route_head* rte) { - char obuff[256]; - char buff1[64], buff2[64]; - char* pbuff; + QScopedPointer obuff; + QString buff1; + QString buff2; + QString* pbuff; QString icon_token; /* count waypoints for this route */ @@ -1497,31 +1499,29 @@ mag_route_trl(const route_head* rte) } if (i == 1) { - pbuff = buff1; + pbuff = &buff1; } else { - pbuff = buff2; + pbuff = &buff2; } // Write name, icon tuple into alternating buff1/buff2 buffer. - sprintf(pbuff, "%s,%s", CSTR(waypointp->shortname), CSTR(icon_token)); + *pbuff = waypointp->shortname + ',' + icon_token; if ((waypointp == rte->waypoint_list.back()) || ((i % 2) == 0)) { - char expbuf[1024]; + QString expbuf; thisline++; - expbuf[0] = 0; if (explorist) { - snprintf(expbuf, sizeof(expbuf), "%s,", - CSTRc(rte->rte_name)); + expbuf = rte->rte_name + ','; } - sprintf(obuff, "PMGNRTE,%d,%d,c,%d,%s%s,%s", + xasprintf(obuff, "PMGNRTE,%d,%d,c,%d,%s%s,%s", numlines, thisline, rte->rte_num ? rte->rte_num : route_out_count, - expbuf, - buff1, buff2); + CSTRc(expbuf), + CSTR(buff1), CSTR(buff2)); - mag_writemsg(obuff); - buff1[0] = '\0'; - buff2[0] = '\0'; + mag_writemsg(obuff.data()); + buff1.clear(); + buff2.clear(); i = 0; } } diff --git a/util.cc b/util.cc index b93c38d2d..4e622fc51 100644 --- a/util.cc +++ b/util.cc @@ -19,36 +19,39 @@ */ -#include // for isspace, isalpha, ispunct, tolower, toupper -#include // for errno -#include // for fabs, floor -#include // for va_list, va_end, va_start, va_copy -#include // for size_t, vsnprintf, FILE, fopen, printf, sprintf, stderr, stdin, stdout -#include // for uint32_t -#include // for abs, getenv, calloc, free, malloc, realloc -#include // for strlen, strcat, strstr, memcpy, strcmp, strcpy, strdup, strchr, strerror -#include // for mktime, localtime - -#include // for QByteArray -#include // for QChar, operator<=, operator>= -#include // for QCharRef -#include // for QDateTime -#include // for QFileInfo -#include // for QList -#include // for QString -#include // for QStringRef -#include // for QTextCodec -#include // for operator<<, QTextStream, qSetFieldWidth, endl, QTextStream::AlignLeft -#include // for QXmlStreamAttribute -#include // for CaseInsensitive -#include // for QTimeZone -#include // for qAsConst, QAddConst<>::Type, qPrintable +#include // for sort +#include // for isspace, isalpha, ispunct, tolower, toupper +#include // for errno +#include // for fabs, floor +#include // for va_list, va_end, va_start, va_copy +#include // for size_t, vsnprintf, FILE, fopen, printf, sprintf, stderr, stdin, stdout +#include // for uint32_t +#include // for abs, getenv, calloc, free, malloc, realloc +#include // for strlen, strcat, strstr, memcpy, strcmp, strcpy, strdup, strchr, strerror +#include // for mktime, localtime + +#include // for QByteArray +#include // for QChar, operator<=, operator>= +#include // for QCharRef +#include // for QDateTime +#include // for QFileInfo +#include // for QList +#include // for QScopedPointer +#include // for QString +#include // for QStringRef +#include // for QTextCodec +#include // for operator<<, QTextStream, qSetFieldWidth, endl, QTextStream::AlignLeft +#include // for QXmlStreamAttribute +#include // for QXmlStreamAttributes +#include // for CaseInsensitive +#include // for QTimeZone +#include // for qAsConst, QAddConst<>::Type, qPrintable #include "defs.h" -#include "cet.h" // for cet_utf8_to_ucs4 -#include "src/core/datetime.h" // for DateTime -#include "src/core/logging.h" // for Warning -#include "src/core/xmltag.h" // for xml_tag, xml_attribute, xml_findfirst, xml_findnext +#include "cet.h" // for cet_utf8_to_ucs4 +#include "src/core/datetime.h" // for DateTime +#include "src/core/logging.h" // for Warning +#include "src/core/xmltag.h" // for xml_tag, xml_attribute, xml_findfirst, xml_findnext // First test Apple's clever macro that's really a runtime test so // that our universal binaries work right. @@ -254,6 +257,20 @@ xasprintf(QString* strp, const char* fmt, ...) return res; } +int +xasprintf(QScopedPointer& strp, const char* fmt, ...) +{ + va_list args; + + va_start(args, fmt); + char* cstrp; + int res = xvasprintf(&cstrp, fmt, args); + strp.reset(cstrp); + va_end(args); + + return res; +} + int xvasprintf(char** strp, const char* fmt, va_list ap) { -- 2.30.2